Hint that memchr returns an in-bounds index - #159784
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @petrochenkov (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? libs |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
…uwer Rollup of 22 pull requests Successful merges: - #158147 (std: fix stack buffer overflow in Windows junction_point) - #159784 (Hint that memchr returns an in-bounds index) - #160130 (Select cache values to verify by key fingerprint, not value fingerprint) - #160343 (Rename `OutlivesPredicate` to `OutlivesClause`) - #160360 (Remove rustc_middle dependency on rustc_hir_pretty) - #160387 (rustc_codegen_ssa: Correctly apply the static `--jobs-backend` limit to backend parallelism) - #160422 (move mir-opt miri tests to CI logic) - #160444 (Avoid resolving path keywords outside `TypeNS`) - #160510 (Resolver: (un)tracked borrows for `CmRefCell` made safe my unsafe speculative flag) - #155424 ([blocked] Link to proposed LLM policy in CONTRIBUTING and pull request template) - #158726 (std: move futex implementations into sys::sync::futex) - #159225 (Split IncrCompSession out of Session) - #159820 (Make the `rustc_unsafe_specialization_marker` attribute actually `unsafe`) - #160198 (Rework `smallest_range_containing` to handle duplicates) - #160362 (Split `SpannedTypeVisitor` into its own crate, `rustc_ty_walk`) - #160390 (autodiff: Handle slice-tailed DSTs in type trees) - #160420 (Suggest `cast_signed()` for overflowing signed integer literals) - #160520 (Add some tests for specialization) - #160522 (fix(bootstrap): Normalize the names of proc macro dependency crates) - #160531 (docs: fix typo in AllowExprMetavar comment) - #160538 (Update expect messages in tcp.rs doc examples to follow the style guide) - #160548 (bootstrap: Register `coverage-map` and `coverage-run` aliases via a separate step)
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #160554 (comment) |
|
This pull request was unapproved. |
|
@JonathanBrouwer Did the rollup run a different set of tests than the try build I ran? |
|
@Darksonn The rollup failed on the |
LLVM 21 preserves the bounds assumption but does not eliminate the aggregate phi that LLVM 22 removes. Check each version's supported optimization and restore the shared postcondition so direct callers can eliminate bounds checks.
|
@bors try jobs=x86_64-gnu-llvm-21-3 |
This comment has been minimized.
This comment has been minimized.
…ds, r=<try> Hint that memchr returns an in-bounds index try-job: x86_64-gnu-llvm-21-3
|
@bors r+ rollup=iffy |
…result-bounds, r=Darksonn Hint that memchr returns an in-bounds index `memchr_aligned` always returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant with `assert_unchecked` so LLVM can simplify callers such as `str::find(char)` In x86_64 codegen, `find(char)` shrank from 463 to 377 bytes.
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #160615 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#160615), which was unapproved. |
There was a problem hiding this comment.
I think we should take this test out. It's too fragile.
View all comments
memchr_alignedalways returns an index within the input slice, but that fact was not visible to callers after inlining. Record the invariant withassert_uncheckedso LLVM can simplify callers such asstr::find(char)In x86_64 codegen,
find(char)shrank from 463 to 377 bytes.